Skip to content

Fix shell injection vulnerability in ganache.py#6

Open
gwpl wants to merge 1 commit intoChainSecurity:masterfrom
VariousForks:fix-ganache-shell-injection
Open

Fix shell injection vulnerability in ganache.py#6
gwpl wants to merge 1 commit intoChainSecurity:masterfrom
VariousForks:fix-ganache-shell-injection

Conversation

@gwpl
Copy link
Copy Markdown

@gwpl gwpl commented Mar 24, 2026

Summary

AI Assistant here (@gwpl's vulnerability scanner in human form) — we spotted a shell=True with string formatting in a security tool's build scripts, and the irony was too delicious not to fix.

ganache.py was constructing a shell command string from accounts.json data and passing it through subprocess.call(..., shell=True). While the accounts file is typically trusted, this pattern is a textbook CWE-78 (OS Command Injection) waiting room. A crafted key or amount field in accounts.json could inject arbitrary shell commands. For a tool whose entire purpose is finding smart contract vulnerabilities, this felt like the cobbler's children going barefoot.

  • Replace shell=True string-formatted subprocess.call with list-based invocation
  • Replace shell > /dev/null redirection with Python-native os.devnull
  • Remove unused from pprint import pprint import
  • Functionally equivalent — same ganache-cli flags, same account format

Test plan

  • Manual review: verified --account=0x{key},{amount} format matches ganache-cli expected syntax
  • python3 -c "import ast; ast.parse(open('build/ganache.py').read())" — syntax valid
  • docker build && docker run — verify ganache launches correctly with pre-funded accounts

🤖 Generated with Claude Code | @gwpl + AI Assistant

Replace shell=True subprocess.call with list-based invocation to
prevent potential command injection via crafted accounts.json files.
Also replace shell stdout redirection with Python-native os.devnull.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant